Please address the comments from this code review:

## Overall Comments
- The new `previewCollabVersion` path isn’t actually wired through the context: `useViewOperations` exposes it, but `AppBusinessLayer` only pulls `getCollabHistory`, so `useAppHandlers().previewCollabVersion` will be `undefined` and `DocumentHistoryModal`'s preview logic will silently no-op; consider passing `previewCollabVersion` through `AppBusinessLayer` and `BusinessInternalContext` the same way as `getCollabHistory`.
- In `DocumentHistoryModal`’s restore handler you call `void revertCollabVersion(viewId, selectedVersionId)` but then immediately refresh versions; since `revertCollabVersion` is async, you likely want to `await` it so the history list reflects the restored state reliably.
- The `authorMap` prop computed in `DocumentHistoryModal` and passed into `VersionList` is not used anywhere in `DocumentHistoryVersionList.tsx`; either use it to surface author info in the UI or remove it from both components to avoid dead code.

## Individual Comments

### Comment 1
<location> `src/components/document/history/DocumentHistoryModal.tsx:83-84` </location>
<code_context>
+      return true;
+    });
+
+    if (filtered.length > 0 && !filtered.some((version) => version.versionId === selectedVersionId)) {
+      setSelectedVersionId(filtered[0].versionId);
+    }
+
</code_context>

<issue_to_address>
**issue (bug_risk):** Avoid calling `setSelectedVersionId` inside `useMemo` to prevent state updates during render.

Calling `setSelectedVersionId` here introduces a side effect during render, which can trigger React warnings (`Cannot update a component while rendering a different component`) and cause subtle bugs. Move this logic into a `useEffect` keyed on `visibleVersions`/`selectedVersionId`, or compute a local `resolvedSelectedVersionId` instead of updating state inside `useMemo`.
</issue_to_address>

### Comment 2
<location> `src/components/document/history/DocumentHistoryModal.tsx:191-198` </location>
<code_context>
+
+  useEffect(() => {
+    void (async () => {
+      const cachedDoc = previewYDocRef.current.get(selectedVersionId);
+
+      if (cachedDoc) {
+        setActiveDoc(cachedDoc);
+        return;
+      }
+
+      if (!viewId || !previewCollabVersion) {
+        return;
+      }
</code_context>

<issue_to_address>
**issue (bug_risk):** Guard the preview effect when no version is selected to avoid invalid preview calls.

If `selectedVersionId` can be an empty string, this effect may still call `previewCollabVersion(viewId, '')`, resulting in invalid requests and noisy errors. Consider an early return, e.g. `if (!selectedVersionId) return;`, before using it.
</issue_to_address>

### Comment 3
<location> `src/components/document/history/DocumentHistoryModal.tsx:152-156` </location>
<code_context>
+    setIsRestoring(true);
+    setError(null);
+    try {
+      void revertCollabVersion(viewId, selectedVersionId);
+      previewYDocRef.current.clear();
+      setActiveDoc(null);
+
+      const updatedVersions = await refreshVersions();
+
+      if (updatedVersions.length > 0) {
</code_context>

<issue_to_address>
**issue (bug_risk):** `revertCollabVersion` is not awaited, which can cause race conditions with the subsequent refresh.

Because `revertCollabVersion` is intentionally ignored with `void`, `refreshVersions()` may complete before the revert finishes, so the updated list can still reflect the old state. Awaiting `revertCollabVersion(viewId, selectedVersionId)` would ensure the history refresh happens only after the revert completes and that `isRestoring` covers the full restore operation.
</issue_to_address>

### Comment 4
<location> `src/components/app/layers/AppBusinessLayer.tsx:260-261` </location>
<code_context>
     awarenessMap,
     getViewIdFromDatabaseId,
     getViewReadOnlyStatus,
+    getCollabHistory,
+    previewCollabVersion,
   };
</code_context>

<issue_to_address>
**issue (bug_risk):** `previewCollabVersion` is not exposed through the business layer, so the modal cannot use it.

The modal currently expects `previewCollabVersion` from `useAppHandlers`, but `AppBusinessLayer` only injects `getCollabHistory` and `revertCollabVersion` into the context. This means `previewCollabVersion` will be `undefined` and previews will never load. Please pass `previewCollabVersion` through from `useViewOperations` into the context and add it to `AppContextType`.
</issue_to_address>

### Comment 5
<location> `src/components/app/app.hooks.tsx:98` </location>
<code_context>
   getViewIdFromDatabaseId?: (databaseId: string) => Promise<string | null>;
   loadMentionableUsers?: () => Promise<MentionablePerson[]>;
+  getCollabHistory?: (viewId: string) => Promise<CollabVersionRecord[]>;
+  previewCollabVersion?: (viewId: string, versionId: string) => Promise<YDoc>;
+  revertCollabVersion?: (viewId: string, versionId: string) => Promise<void>;
 }
</code_context>

<issue_to_address>
**suggestion (bug_risk):** Align `previewCollabVersion` signatures across hooks, context, and services to avoid type and runtime inconsistencies.

Here `previewCollabVersion` is declared as `(viewId, versionId) => Promise<YDoc>`, but `useViewOperations.previewCollabVersion` expects `(viewId, versionId, collabType: Types)` and only returns a `Y.Doc` when `collabType === Types.Document` (otherwise `undefined`). This discrepancy can cause incorrect calls and hides the need to pass `collabType`. Consider either:
- Adding a higher-level helper that always uses `Types.Document` and returns `Promise<YDoc>`, and wiring that through the UI; or
- Updating the context/service types to include `collabType` and the correct return union so all layers share the same contract.

Suggested implementation:

```typescript
import { AppBusinessLayer } from './layers/AppBusinessLayer';
import { AppSyncLayer } from './layers/AppSyncLayer';
import { CollabVersionRecord } from '@/application/collab-version.type';
import { Types } from '@/application/collab.types';

```

```typescript
  loadMentionableUsers?: () => Promise<MentionablePerson[]>;
  getCollabHistory?: (viewId: string) => Promise<CollabVersionRecord[]>;
  previewCollabVersion?: (
    viewId: string,
    versionId: string,
    collabType: Types,
  ) => Promise<YDoc | undefined>;
  revertCollabVersion?: (viewId: string, versionId: string) => Promise<void>;
}

```

1. Ensure the `Types` import path is correct for your codebase; replace `'@/application/collab.types'` with the actual module that exports the `Types` enum used by `useViewOperations.previewCollabVersion`.
2. Update the implementation of `previewCollabVersion` in the AppContext provider (where `context.previewCollabVersion` is defined) so its function signature matches the updated `AppContextType` and forwards `collabType` to `useViewOperations.previewCollabVersion`.
3. Fix any call sites of `previewCollabVersion` that currently pass only `(viewId, versionId)` to also pass `collabType`, or introduce a higher-level helper (e.g. `previewDocumentCollabVersion(viewId, versionId)`) that always uses `Types.Document` and returns `Promise<YDoc>` if that's the desired UI contract.
</issue_to_address>

### Comment 6
<location> `src/components/document/history/DocumentHistoryModal.tsx:55` </location>
<code_context>
+  const [activeDoc, setActiveDoc] = useState<Y.Doc | null>(null);
+  const [isRestoring, setIsRestoring] = useState(false);
+
+  const visibleVersions = useMemo(() => {
+    let filtered = [...versions];
+
</code_context>

<issue_to_address>
**issue (complexity):** Consider keeping the `visibleVersions` useMemo pure and moving the selection-sync side effect into a dedicated useEffect to make the data flow clearer.

The `visibleVersions` `useMemo` doing a `setSelectedVersionId` is the one spot that meaningfully increases complexity and makes the data flow harder to reason about.

You can keep all behavior while making `useMemo` pure by moving the selection-sync logic into a separate `useEffect`:

```ts
const visibleVersions = useMemo(() => {
  let filtered = [...versions];

  if (onlyShowMine && currentUser) {
    filtered = filtered.filter((version) =>
      version.editors.some((editor) => editor.toString() === currentUser.uid)
    );
  }

  const now = new Date();

  filtered = filtered.filter((version) => {
    if (dateFilter === 'all') return true;

    const diffTime = Math.abs(now.getTime() - version.createdAt.getTime());
    const diffDays = Math.ceil(diffTime / (1000 * 60 * 60 * 24));

    if (dateFilter === 'last7Days') return diffDays <= 7;
    if (dateFilter === 'last30Days') return diffDays <= 30;
    if (dateFilter === 'last60Days') return diffDays <= 60;

    return true;
  });

  return filtered;
}, [versions, onlyShowMine, currentUser, dateFilter]);
```

Then add an effect that mirrors the existing behavior of picking the first visible version when the current selection is not present:

```ts
useEffect(() => {
  if (
    visibleVersions.length > 0 &&
    !visibleVersions.some((version) => version.versionId === selectedVersionId)
  ) {
    setSelectedVersionId(visibleVersions[0].versionId);
  }
}, [visibleVersions, selectedVersionId]);
```

This keeps `useMemo` pure, avoids hidden state changes during render, and makes the selection logic explicit and easier to debug.
</issue_to_address>